Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resolver: document handling UpdateState errors by resolvers #6002

Merged
merged 1 commit into from
Feb 8, 2023

Conversation

fho
Copy link
Contributor

@fho fho commented Feb 1, 2023

Extend the Godoc for resolver.ClientConn.UpdateState with a description of how resolvers should handle returned errors.

The description is based on the explanation of dfawley in #5048

RELEASE NOTES:

  • resolver: document expected error handling of UpdateState errors

Copy link
Member

@dfawley dfawley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates!

resolver/resolver.go Outdated Show resolved Hide resolved
resolver/resolver.go Outdated Show resolved Hide resolved
resolver/resolver.go Outdated Show resolved Hide resolved
resolver/resolver.go Outdated Show resolved Hide resolved
@fho
Copy link
Contributor Author

fho commented Feb 2, 2023

@dfawley thanks for the feedback, I adapted it to your suggestions, please have a look again

Extend the Godoc for resolver.ClientConn.UpdateState with a
description of how resolvers should handle returned errors.

The description is based on the explanation of dfawley in
grpc#5048
Copy link
Member

@dfawley dfawley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! For future reference, please do not perform force-pushes when updating PR comments, as this messes up github review comments and loses the history of the PR as it changed over time. This one was small enough that it doesn't matter, but it's a good general practice to follow.

Thanks for the change!

@dfawley dfawley assigned zasweq and unassigned dfawley Feb 8, 2023
@dfawley dfawley requested a review from zasweq February 8, 2023 21:13
@dfawley
Copy link
Member

dfawley commented Feb 8, 2023

@zasweq please double-check and approve so we can merge this, thanks.

Copy link
Contributor

@zasweq zasweq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks.

@zasweq zasweq merged commit 55dfae6 into grpc:master Feb 8, 2023
1 check passed
@fho fho deleted the resolver_doc branch March 22, 2023 09:27
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Documentation Documentation or examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants